gh-140928: Add a constant pool to all executors; Contant propagate using the pool#140968
gh-140928: Add a constant pool to all executors; Contant propagate using the pool#140968Fidget-Spinner wants to merge 3 commits intopython:mainfrom
Conversation
| int | ||
| _Py_uop_promote_to_constant_pool(JitOptContext *ctx, PyObject *obj) | ||
| { | ||
| return PyList_Append(ctx->constant_pool, obj); |
There was a problem hiding this comment.
Maybe we should add an assert here, like assert(ctx->contstant_pool)?
There was a problem hiding this comment.
PyList_Append itself already checks that it's a list.
There was a problem hiding this comment.
Yes, but no. If you pass a NULL pointer to PyList_Append it crashes.
I'm sure there is no chances to get NULL pointer here. But I believe extra assert will not hurt.
|
I'm not sure my thought is right. Correct me plz if I'm wrong. I think we don't limit the pool size, right? Will this cause a memory leak or not? And I think this is a append only list for now. I'm not sure we need to think about if the user delete some const variable and we need to add a pop out path |
If we're careful about what we add to the pool, there won't be a memory leak.
If the user deletes a constant variable, the variable will stay alive in the pool for now. This is fine as the pool only contains constants which don't take a lot of memory. |
I see, this is making sense. Thanks for your work!(lol |
This PR promotes constant propagated objects into a pool. Storing them there. We already see this affecting the optimizations in the test suite, as more things are constant propagated away!
This sets up the infrastructure to promote more things in the future should we feel like it (e.g. function, code objects).